fix(proxy): model-level guardrails not executing for non-streaming post_call#23774
Conversation
…st_call Model-level guardrails (litellm_params.guardrails on a deployment) were only merged into request metadata in the streaming post_call path (async_post_call_streaming_hook) but not in the non-streaming path (post_call_success_hook). This caused should_run_guardrail to skip the guardrail because the guardrail name was never added to metadata.guardrails. Add the same _check_and_merge_model_level_guardrails call to post_call_success_hook before the guardrail callback loop, mirroring the streaming path. Fixes model-level guardrails silently not firing for non-streaming post_call requests.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR fixes a bug where model-level guardrails configured via Key changes:
Confidence Score: 4/5
|
| Filename | Overview |
|---|---|
| litellm/proxy/utils.py | Adds _check_and_merge_model_level_guardrails() call in post_call_success_hook before the guardrail callback loop, mirroring the streaming path. The fix is correct and targeted; a minor inefficiency exists in calling the merge unconditionally even when no guardrail callbacks are registered. |
| tests/test_litellm/proxy/test_model_level_guardrails.py | New test file with 8 unit tests for _check_and_merge_model_level_guardrails and 2 integration tests covering the non-streaming hook path. Tests are mock-only (no real network calls), use __file__-relative path resolution, and cover the key positive and negative cases. |
Sequence Diagram
sequenceDiagram
participant Client
participant ProxyLogging
participant _check_and_merge
participant Router
participant CustomGuardrail
Client->>ProxyLogging: POST /chat/completions (non-streaming)
ProxyLogging->>ProxyLogging: Classify callbacks into guardrail_callbacks / other_callbacks
Note over ProxyLogging: NEW (this PR): merge model-level guardrails
ProxyLogging->>_check_and_merge: _check_and_merge_model_level_guardrails(data, llm_router)
_check_and_merge->>Router: get_deployment(model_id)
Router-->>_check_and_merge: deployment (with litellm_params.guardrails)
_check_and_merge->>_check_and_merge: merge guardrails into metadata
_check_and_merge-->>ProxyLogging: guardrail_data (merged)
loop for each CustomGuardrail
ProxyLogging->>CustomGuardrail: should_run_guardrail(data=guardrail_data, post_call)
CustomGuardrail-->>ProxyLogging: True / False
alt should run
ProxyLogging->>CustomGuardrail: async_post_call_success_hook(data, response)
CustomGuardrail-->>ProxyLogging: guardrail_response
end
end
ProxyLogging-->>Client: response
Last reviewed commit: b0c317e
| import pytest | ||
| from unittest.mock import MagicMock, patch | ||
|
|
||
| sys.path.insert(0, os.path.abspath("../../..")) |
There was a problem hiding this comment.
Fragile relative sys.path manipulation
os.path.abspath("../../..") resolves relative to the process working directory at the time the test is collected, not relative to the test file's location. If pytest is run from any directory other than tests/test_litellm/proxy/, this path will resolve to the wrong location and the subsequent imports will fail or silently import from an unexpected location.
Use __file__-relative path resolution instead, which is stable regardless of the working directory:
| sys.path.insert(0, os.path.abspath("../../..")) | |
| sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), "../../.."))) |
Address Greptile review — resolve sys.path relative to the test file location instead of the process working directory.
| guardrail_data = _check_and_merge_model_level_guardrails( | ||
| data=data, llm_router=llm_router |
There was a problem hiding this comment.
Merge called even when no guardrail callbacks exist
_check_and_merge_model_level_guardrails() is invoked unconditionally after the callback-classification loop, even when guardrail_callbacks is empty (i.e., no CustomGuardrail is registered). This causes an unnecessary llm_router.get_deployment() lookup and dict copy on every non-streaming request, even when there are no guardrail callbacks to trigger.
Guard the call so it only runs when there is at least one guardrail callback to consider:
| guardrail_data = _check_and_merge_model_level_guardrails( | |
| data=data, llm_router=llm_router | |
| if guardrail_callbacks: | |
| guardrail_data = _check_and_merge_model_level_guardrails( | |
| data=data, llm_router=llm_router | |
| ) | |
| else: | |
| guardrail_data = data | |
Model-level guardrails (litellm_params.guardrails on a deployment) were only merged into request metadata in the streaming post_call path (async_post_call_streaming_hook) but not in the non-streaming path (post_call_success_hook). This caused should_run_guardrail to skip the guardrail because the guardrail name was never added to metadata.guardrails.
Add the same _check_and_merge_model_level_guardrails call to post_call_success_hook before the guardrail callback loop, mirroring the streaming path.
Fixes model-level guardrails silently not firing for non-streaming post_call requests.
Relevant issues
Fixes #18363 (partially — the original fix PR #18895 only covered the pre_call path)
Pre-Submission checklist
tests/test_litellm/directory, Adding at least 1 test is a hard requirement - see detailsmake test-unit@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewType
🐛 Bug Fix
✅ Test
Changes
Problem
Model-level guardrails configured via
litellm_params.guardrailsin the config or UI were not being applied for non-streamingpost_callrequests. The guardrail only fired when explicitly passed in the request body or when using streaming.Root Cause
_check_and_merge_model_level_guardrails()(which readsdeployment.litellm_params.guardrailsand merges them intometadata.guardrails) was called inasync_post_call_streaming_hook(utils.py:2097) but not inpost_call_success_hook(utils.py:1934). Without the merge,should_run_guardrail()found no matching guardrail name in metadata and returnedFalse.Fix
Added the same
_check_and_merge_model_level_guardrails()call topost_call_success_hookbefore the guardrail callback loop, mirroring the streaming path.Files Changed
litellm/proxy/utils.py— Added model-level guardrail merge inpost_call_success_hook(+5 lines)tests/test_litellm/proxy/test_model_level_guardrails.py— New test file: 8 unit tests for the merge function + 2 integration tests for the hook